r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Oops. This is not going to work without flagging every cargo doc test case nightly-only 🥲

@rfcbot fcp merge T-cargo

@weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

• @0xPoe
• @Eh2406
• @Muscraft
• @arlosi
• @ehuss
• @epage
• @joshtriplett
• @weihanglo

Concerns:

• comma-in-path (fix: stabilize -Zrustdoc-depinfo #17020 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

I found one regression case from this feature:

Problem

cargo doc fails when the target directory path contains a comma.

Cargo places rustdoc's dep-info file under its private fingerprint directory. It then embeds that path into rustdoc's comma-separated --emit argument:

--emit=html-static-files,html-non-static-files,dep-info=/path/to/target,comma/debug/.fingerprint/... /doc-lib-foo.d                                                                                       

Rustdoc splits the entire value on commas, causing the portion after the comma in the path to be interpreted as another emission type:

 error: unrecognized emission type: comma/debug/.fingerprint/...                                    

A comma is valid in a target directory name, and the same cargo doc command succeeds when rustdoc dep-info is not enabled.

Expected behavior (if you are using the current PR, there is no need to add the -Zrustdoc-depinfo):

 cargo doc -Zrustdoc-depinfo --target-dir target,comma                                              

should succeed and generate:

 target,comma/doc/comma_target/index.html                                                           

Actual behavior: it will fail with an unrecognized emission type error.

Steps

 $ cargo new --lib --name comma_target cargo-rustdoc-depinfo-comma-repro                            
 $ cd cargo-rustdoc-depinfo-comma-repro                                                             
 $ cargo +nightly doc -Zrustdoc-depinfo --target-dir 'target,comma' -vv                             

For comparison, this succeeds:

 $ cargo +nightly doc --target-dir 'target,comma'                                                   

I think this is a very corner case, so I’m not sure if it’s really worth fixing. But it does look like a regression, although I guess not many people would put a comma in the path. So I’m not sure, @weihanglo, what do you think? And do we need to bother fixing this?

@rfcbot concern comma-in-path

Filed rust-lang/rust#159743

One workaround: fall back to the old file walk solution when comma is detected.